fix dashboard index pattern race condition #72899

Dosant · 2020-07-22T16:23:34Z

Summary

Fixes #72601 - "Sometimes filters on non-default-indexpattern dashboard are stuck in warning state".

Please note: it doesn't fix flash of warning state during dashboard loading: #72899 (review). It makes sure we won't stuck in this state. Separate issue to address flickering: #73286

Looks like we hard race condition there for a long time, but noticed it because of #66979 which made it easily noticeable

Problem

There was a race condition when setting index pattern in a dashboard:

Old code:


function updateIndexPattern() {
 const indexPatternsFromPanels = getIndexPatternsFromPanels();

 if (indexPatternsFromPanels.length > 0) {
    setIndexPatterns(indexPatternsFromPanels.length)
 } else {
  fetchDefaultIndexPatterns()
   .then(setIndexPatterns)
 }
}

And this function was called a bunch of times in the beginning.
On each getOutput$() change:

updateIndexPattern();
updateIndexPattern();
updateIndexPattern(); // assume on this point indexPatternsInPanels are resolved and available
updateIndexPattern();
updateIndexPattern();

⬆️ but because code inside fetches defaults async, it could have happened that earlier calls finish last

Solution

I refactored that piece to use RxJS to get handy cancelations.

I didn't add any test, as not sure about the good way to test this. This fixes a race condition which would be hard to reliably catch in a function test. And dashboard controller need a refactor to start adding unit tests for logic there.

Release notes

Sometimes when creating filters on a dashboard suggestions from default index patterns were shown by mistake.

elasticmachine · 2020-07-22T16:25:40Z

Pinging @elastic/kibana-app-arch (Team:AppArch)

flash1293

This looks pretty good already and it seems like I'm always ending up in the right state, but during loading it shows the warnings the wrong way:

Maybe we should pass a "loading" flag of some kind in to prevent "Warning" flags as longs as index patterns are still fetched?

…oard-index-pattern-race-condition

Dosant · 2020-07-23T11:43:36Z

@flash1293, what if do not show filter bar until index patterns are properly loaded?
(introducing new loading state for filter bar in a sake of this hot fix seems like a risky overkill)

flash1293 · 2020-07-23T11:47:15Z

@Dosant Just realized you want to get this into 7.9, not showing the filter bar is fine for me then. Maybe we can add a nice loading state (Maybe a Loading filters...) in a later version

Dosant · 2020-07-23T12:17:50Z

Hm, not sure I like this "hotfix" because of context shift:

I am thinking to leave this fix for 7.9 focused on fixing index pattern resolution race condition (no hiding filters bar) and to create a ticket for follow up with proper loading state for filters. WDYT?

flash1293 · 2020-07-23T12:23:37Z

@Dosant Can we hold back the filters till the index patterns are loaded? So rendering an empty filter bar, then passing the filters through once the index patterns are ready?

If that doesn't fit well I would be fine with the current state.

Dosant · 2020-07-23T12:27:26Z

@flash1293, I'd say doesn't fit :(, filters and index patterns get into filter bar in completely different code paths.

Index patterns via react component props
Filters via FilterManager service which get's syncing with DashboardStateManger (url)

flash1293 · 2020-07-23T14:32:35Z

@Dosant I'm fine with the current state in that case. @majagrubic are you fine with that approach for now as well? We can improve in 7.10

Dosant · 2020-07-27T07:37:21Z

@elasticmachine merge upstream

…tion

flash1293

@majagrubic is out this week, it seems like a pretty risky change for 7.9 at this point. @Dosant what do you think about moving this to 7.9.1?

flash1293

Approving as this is looking fine for me (with room for improvement as discussed) - we can already prepare the backports till we have a decision how to handle it

lizozom · 2020-07-27T13:45:11Z

I strongly support fixing this bug and handling loading state during 7.10.
Feel free to create an issue and assign it to me.

src/plugins/dashboard/public/application/dashboard_app_controller.tsx

lizozom

Added some minor comments.

src/plugins/dashboard/public/application/dashboard_app_controller.tsx

…ub.com:Dosant/kibana into dev/fix-dashboard-index-pattern-race-condition

Dosant · 2020-07-27T14:32:36Z

@lizozom, created an issue to address "flickering" #73286

Dosant · 2020-07-28T05:55:06Z

@elasticmachine merge upstream

…tion

kibanamachine · 2020-07-28T07:45:46Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 98dbd27

Build metrics

page load bundle size

id	value	diff	baseline
dashboard	679.5KB	+697.0B	678.9KB

History

💚 Build #64336 succeeded 14d8ec5
💚 Build #64247 succeeded 7f10d38
💚 Build #63669 succeeded 7b86bef

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* fix dashboard index pattern race condition * improve Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* fix dashboard index pattern race condition * improve Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* master: (44 commits) [Search] add server logs (elastic#72454) [SIEM][Timelines] Updates timeline template callout text (elastic#73334) Fix App status flaky test (elastic#72853) [Functional Tests] Increase the timeout when locating the tableview] (elastic#73243) Use "Apply_filter_trigger" in dashboard drilldown (elastic#71468) fix dashboard index pattern race condition (elastic#72899) [Functional Tests] Increase waitTime for timelion to fetch the results (elastic#73255) [Functional Tests] Fix flakiness on TSVB chart on switching index patterns test (elastic#73238) updates cypress to v4.11.0 (elastic#73327) [Metrics UI] Saved views bugs (elastic#72518) [Ingest Manager] Convert select agent config step to use combo box (elastic#73172) Exclude `version` from package config attributes that are copied, add safeguard to package config bulk create (elastic#73128) [Security Solution][ML] Updates siem group name to security (elastic#73218) [Security Solution] Show proper icon for termination status of all processes (elastic#73235) [Security Solution][Resolver] Show origin node details in panel on load (elastic#73313) [Security solution] Threat hunting test coverage improvements (elastic#73276) [Security Solution][Exceptions] - Update exception item comments to include id (elastic#73129) [Enterprise Search] Error state UI tweaks to account for current Cloud SSO behavior (elastic#73324) [dev/build/docker_generator] convert to typescript (elastic#73339) [APM] Fix focus map link on service map (elastic#73338) ...

fix dashboard index pattern race condition

7b86bef

Dosant requested a review from a team July 22, 2020 16:23

Dosant added the Team:AppArch label Jul 22, 2020

Dosant added bug Fixes for quality problems that affect the customer experience v7.9.0 labels Jul 22, 2020

Dosant added the Feature:Dashboard Dashboard related features label Jul 22, 2020

Dosant requested review from lizozom, flash1293 and majagrubic July 22, 2020 16:26

Dosant added release_note:skip Skip the PR/issue when compiling release notes release_note:fix and removed release_note:skip Skip the PR/issue when compiling release notes labels Jul 22, 2020

Dosant marked this pull request as draft July 22, 2020 16:26

Dosant marked this pull request as ready for review July 23, 2020 08:43

flash1293 reviewed Jul 23, 2020

View reviewed changes

Merge branch 'master' of github.com:elastic/kibana into dev/fix-dashb…

0a17b40

…oard-index-pattern-race-condition

Merge branch 'master' into dev/fix-dashboard-index-pattern-race-condi…

7f10d38

…tion

flash1293 reviewed Jul 27, 2020

View reviewed changes

Dosant requested a review from ThomThomson July 27, 2020 09:29

flash1293 approved these changes Jul 27, 2020

View reviewed changes

lizozom reviewed Jul 27, 2020

View reviewed changes

src/plugins/dashboard/public/application/dashboard_app_controller.tsx Outdated Show resolved Hide resolved

lizozom approved these changes Jul 27, 2020

View reviewed changes

src/plugins/dashboard/public/application/dashboard_app_controller.tsx Show resolved Hide resolved

src/plugins/dashboard/public/application/dashboard_app_controller.tsx Show resolved Hide resolved

Dosant added 2 commits July 27, 2020 16:10

Merge branch 'dev/fix-dashboard-index-pattern-race-condition' of gith…

d43edd1

…ub.com:Dosant/kibana into dev/fix-dashboard-index-pattern-race-condition

improve

14d8ec5

Dosant mentioned this pull request Jul 27, 2020

Flickering warning filters state on a dashboard #73286

Closed

Dosant added v7.10.0 v8.0.0 labels Jul 27, 2020

Merge branch 'master' into dev/fix-dashboard-index-pattern-race-condi…

98dbd27

…tion

Dosant merged commit 9b570a9 into elastic:master Jul 28, 2020

Dosant mentioned this pull request Jul 28, 2020

[7.x] fix dashboard index pattern race condition (#72899) #73408

Merged

Dosant added a commit to Dosant/kibana that referenced this pull request Jul 28, 2020

fix dashboard index pattern race condition (elastic#72899)

0101858

* fix dashboard index pattern race condition * improve Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Dosant mentioned this pull request Jul 28, 2020

[7.9] fix dashboard index pattern race condition (#72899) #73409

Merged

Dosant added a commit to Dosant/kibana that referenced this pull request Jul 28, 2020

fix dashboard index pattern race condition (elastic#72899)

e8b46f3

* fix dashboard index pattern race condition * improve Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix dashboard index pattern race condition #72899

fix dashboard index pattern race condition #72899

Dosant commented Jul 22, 2020 •

edited

Loading

elasticmachine commented Jul 22, 2020

flash1293 left a comment

Dosant commented Jul 23, 2020 •

edited

Loading

flash1293 commented Jul 23, 2020

Dosant commented Jul 23, 2020

flash1293 commented Jul 23, 2020

Dosant commented Jul 23, 2020

flash1293 commented Jul 23, 2020

Dosant commented Jul 27, 2020

flash1293 left a comment

flash1293 left a comment

lizozom commented Jul 27, 2020

lizozom left a comment

Dosant commented Jul 27, 2020

Dosant commented Jul 28, 2020

kibanamachine commented Jul 28, 2020

fix dashboard index pattern race condition #72899

fix dashboard index pattern race condition #72899

Conversation

Dosant commented Jul 22, 2020 • edited Loading

Summary

Problem

Solution

Release notes

elasticmachine commented Jul 22, 2020

flash1293 left a comment

Choose a reason for hiding this comment

Dosant commented Jul 23, 2020 • edited Loading

flash1293 commented Jul 23, 2020

Dosant commented Jul 23, 2020

flash1293 commented Jul 23, 2020

Dosant commented Jul 23, 2020

flash1293 commented Jul 23, 2020

Dosant commented Jul 27, 2020

flash1293 left a comment

Choose a reason for hiding this comment

flash1293 left a comment

Choose a reason for hiding this comment

lizozom commented Jul 27, 2020

lizozom left a comment

Choose a reason for hiding this comment

Dosant commented Jul 27, 2020

Dosant commented Jul 28, 2020

kibanamachine commented Jul 28, 2020

💚 Build Succeeded

Build metrics

page load bundle size

History

Dosant commented Jul 22, 2020 •

edited

Loading

Dosant commented Jul 23, 2020 •

edited

Loading